Skip to content

Comments

Update LDK dependency for splicing, http, and wallet_utils#794

Open
jkczyz wants to merge 4 commits intolightningdevkit:mainfrom
jkczyz:2026-02-splicing-api-change
Open

Update LDK dependency for splicing, http, and wallet_utils#794
jkczyz wants to merge 4 commits intolightningdevkit:mainfrom
jkczyz:2026-02-splicing-api-change

Conversation

@jkczyz
Copy link
Contributor

@jkczyz jkczyz commented Feb 18, 2026

The LDK dependency bump introduced a new splicing API that separates negotiation from coin selection (lightningdevkit/rust-lightning#4290), letting LDK handle transaction construction internally rather than requiring manual UTXO selection and change address management.

Also handles changes to lightning-block-sync moving to bitreq (lightningdevkit/rust-lightning#4350), making a test macro a function (lightningdevkit/rust-lightning#4425), and moving some coin selection utils to wallet_utils (lightningdevkit/rust-lightning#4382).

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Feb 18, 2026

👋 Thanks for assigning @tnull as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

Ok(txid)
}

pub(crate) fn select_confirmed_utxos(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tnull I'm wondering if we should implement CoinSelectionSource and use this rather than using LdkWallet for UTXO selection. It would prevent double persistence when we need a change output and the need for an explicit persist method after selection.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that seems preferable, if we have everything we need in the API by now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I think the question may be whether we use our own CoinSelectionSource for anchor bumping, too. It depends on how UTXO locking will work. LdkWallet manages whether or not to try to double-spend based on the claim id, whereas for splicing we may accidentally double spend one of those if using our own CoinSelectionSource. Pushed a fixup commit implementing CoinSelectionSource for splicing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, well the branch shows the fixup. Not sure why the PR hasn't updated.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I think the question may be whether we use our own CoinSelectionSource for anchor bumping, too.

It would be great to reuse the same codepaths, IMO. That is, if the API now permits for it.

It depends on how UTXO locking will work. LdkWallet manages whether or not to try to double-spend based on the claim id, whereas for splicing we may accidentally double spend one of those if using our own CoinSelectionSource.

Right, we're still waiting on bitcoindevkit/bdk_wallet#259 to ship with BDK 3.0. For now I'm not sure if we'd then need to also do some tracking based on ClaimId.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed offline. We'll use the current approach and then once BDK ships a new release we can revisit coin selection for anchor bumping.

Alternatives would be to either (1) drop the fixup such that we use LdkWallet for both coin selections or (2) maintain a locked UTXO list for splice coin selection and filter WalletSource::list_confirmed_utxos so that anchor bumping doesn't double spend splicing.

Ok(txid)
}

pub(crate) fn select_confirmed_utxos(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that seems preferable, if we have everything we need in the API by now?

@jkczyz jkczyz force-pushed the 2026-02-splicing-api-change branch from 4c01c07 to 4891c0b Compare February 18, 2026 18:04
@jkczyz jkczyz self-assigned this Feb 19, 2026
@jkczyz jkczyz force-pushed the 2026-02-splicing-api-change branch 2 times, most recently from 7ccaa2a to 01102e6 Compare February 20, 2026 06:03
@jkczyz jkczyz marked this pull request as ready for review February 20, 2026 06:03
@jkczyz jkczyz requested a review from tnull February 20, 2026 06:13
Copy link
Collaborator

@tnull tnull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems this should be based on lightningdevkit/rust-lightning#4382 as it now has landed, as is the next related breaking change?

Copy link
Collaborator

@tnull tnull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM, but as mentioned above it would be good to add a second commit to bump once more and include latest wallet-related API fixes, so that after this PR we can build against current LDK main again.

fn sign_psbt<'a>(
&'a self, psbt: Psbt,
) -> impl Future<Output = Result<Transaction, ()>> + Send + 'a {
debug_assert!(false);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this unreachable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's only used for anchor bumping. For splicing, we provide signatures when handling FundingTransactionReadyForSigning and calling ChannelManager::funding_transaction_signed.

@jkczyz
Copy link
Contributor Author

jkczyz commented Feb 20, 2026

Seems this should be based on lightningdevkit/rust-lightning#4382 as it now has landed, as is the next related breaking change?

Yeah, unfortunately lightningdevkit/rust-lightning#4350 landed between the two PRs. Working on updating now.

jkczyz and others added 2 commits February 20, 2026 10:13
Update splice_in/splice_out to use new LDK two-phase funding API

The LDK dependency bump introduced a new splicing API that separates
negotiation from coin selection, letting LDK handle transaction
construction internally rather than requiring manual UTXO selection
and change address management.

Generated with the assistance of AI (Claude Code).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The latest LDK dependency bump replaced the custom HTTP implementation
in lightning-block-sync with the `bitreq` crate, changing constructor
signatures, error types, and response parsing constraints. Update the
bitcoind chain source to use the new string-based URL constructors and
structured error enums.

Generated with the assistance of AI (Claude Code).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@jkczyz jkczyz force-pushed the 2026-02-splicing-api-change branch from 01102e6 to f78aa6c Compare February 20, 2026 16:33
@jkczyz jkczyz changed the title Update splice_in/splice_out to use new LDK two-phase funding API Update LDK dependency for splicing, http, and wallet_utils Feb 20, 2026
@jkczyz
Copy link
Contributor Author

jkczyz commented Feb 20, 2026

Ok, I changed the branch I used for updating bitcoin-payment-instructions to be based on your branch. As a result the Cashu changes are no longer needed. Also, I made three separate changes to handle updates for lightningdevkit/rust-lightning#4290, lightningdevkit/rust-lightning#4350, and lightningdevkit/rust-lightning#4382. I didn't write the middle PR, so please take a close look at what Claude did to upgrade.

@jkczyz
Copy link
Contributor Author

jkczyz commented Feb 20, 2026

May need to rebase to fix CI. Looking...

@jkczyz
Copy link
Contributor Author

jkczyz commented Feb 20, 2026

May need to rebase to fix CI. Looking...

Ah, actually looks like we still need to update for lightningdevkit/rust-lightning#4425. Will update shortly.

The macro was removed in favor of a function.
Some types previously existing in lightning::events::bump_transaction
have been moved to lightning::util::wallet_utils.
@jkczyz jkczyz force-pushed the 2026-02-splicing-api-change branch from f78aa6c to ad6c554 Compare February 20, 2026 17:14
@jkczyz jkczyz requested a review from tnull February 20, 2026 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants